Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

added alternative embedding models for sentence transformers and openai #101

Merged
merged 1 commit into from
Sep 1, 2023
Merged

added alternative embedding models for sentence transformers and openai #101

merged 1 commit into from
Sep 1, 2023

Conversation

jamescalam
Copy link
Contributor

Addressing #99. Added ability to choose embedding model via the config.yaml. Also added OpenAI API as option (alongside Sentence Transformers). We can do:

models:
  - type: main
    engine: openai
    model: text-davinci-003
  - type: embedding
    engine: openai
    model: text-embedding-ada-002

and

models:
  - type: main
    engine: openai
    model: text-davinci-003
  - type: embedding
    engine: SentenceTransformers
    model: all-MiniLM-L6-v2

I understand there is also some work ongoing in this space, but in a private repo, naturally, I can't see this, so I apologize if any of this conflicts with or overlaps. Let me know if any of this can be of use!

Thanks

This was referenced Aug 22, 2023
Closed
Closed
@drazvan drazvan self-assigned this Aug 22, 2023
@drazvan drazvan added the enhancement New feature or request label Aug 22, 2023
@drazvan
Copy link
Collaborator

drazvan commented Aug 22, 2023

Thanks for this @jamescalam! I'd like to merge this in the next few days. Let me check against the internal work, but I think we'll first merge this and then apply the rest of the changes.

A couple points:

  1. After merge, we'll likely change the interface to be async. Otherwise, in a server scenario whenever an embedding is computed using an endpoint, like with OpenAI, everything is blocked.
  2. We should probably add a register_embedding_provider method as well so that a user can register a custom one from their config.

Nice work! 👍

@drazvan drazvan added this to the v0.5.0 milestone Aug 22, 2023
@drazvan drazvan merged commit 2d0ace1 into NVIDIA:main Sep 1, 2023
1 check failed
@krannnn krannnn mentioned this pull request Dec 4, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@drazvan drazvan

Labels
enhancement New feature or request
Projects
None yet
Milestone
v0.5.0
Development

Successfully merging this pull request may close these issues.
2 participants
@jamescalam @drazvan